Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for a minimum TTL for events #294

Merged
merged 1 commit into from
Jun 29, 2024
Merged

Add support for a minimum TTL for events #294

merged 1 commit into from
Jun 29, 2024

Conversation

smortex
Copy link
Member

@smortex smortex commented Jun 6, 2024

When running with riemann-wrappers, one may want to run different tools with distinct ttl, but the --ttl flag cannot be specified twice, so the following configuration where we want to run the health tool every 5 seconds (default value) but with a longer TTL than the default (twice the interval) will not work:

---
options: --ttl 300
#        ^^^^^^^^^
tools:
- name: "health"
- name: "rdap"
  options: "--interval 21600 --ttl 86400 --domains example.com example.net"
#                            ^^^^^^^^^^^

A workaround is to add an explicit ttl for each tool, but it is not convenient.

Introduce a --minimum-ttl flag that can be used as a global option to circumvent this issue:

---
options: --minimum-ttl 300
tools:
- name: "health"
- name: "rdap"
  options: "--interval 21600 --ttl 86400 --domains example.com example.net"

@smortex smortex added the enhancement New feature or request label Jun 6, 2024
@smortex smortex marked this pull request as draft June 6, 2024 19:04
@smortex smortex force-pushed the add-minimum-ttl branch 3 times, most recently from 46a3a33 to 5f7bad0 Compare June 6, 2024 19:24
@smortex smortex marked this pull request as ready for review June 6, 2024 19:41
jamtur01
jamtur01 previously approved these changes Jun 29, 2024
Copy link
Member

@jamtur01 jamtur01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@smortex smortex dismissed jamtur01’s stale review June 29, 2024 10:44

The merge-base changed after approval.

@jamtur01 jamtur01 self-requested a review June 29, 2024 10:45
jamtur01
jamtur01 previously approved these changes Jun 29, 2024
Copy link
Member

@jamtur01 jamtur01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@smortex smortex dismissed jamtur01’s stale review June 29, 2024 12:48

The merge-base changed after approval.

@smortex
Copy link
Member Author

smortex commented Jun 29, 2024

The merge-base changed after approval.

Meh! I will rebase the branches on top of main, I hope this will help!

When running with riemann-wrappers, one may want to run different tools
with distinct ttl, but the `--ttl` flag cannot be specified twice, so
the following configuration will not work:

```
---
options: --ttl 300
tools:
- name: "health"
- name: "rdap"
  options: "--interval 21600 --ttl 86400 --domains example.com example.net"
```

A workaround is to add an explicit ttl for each tool, but it is not
convenient.

Introduce a `--minimum-ttl` flag that can be used as a global option to
circumvent this issue.
Copy link
Member

@jamtur01 jamtur01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jamtur01 jamtur01 merged commit 649770d into main Jun 29, 2024
7 checks passed
@smortex smortex deleted the add-minimum-ttl branch July 2, 2024 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants